Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mcv/docs/no-gpu-usage.md`:
- Around line 117-118: The examples use the unpublished image tag
"quay.io/gkm/mcv:full"; update those occurrences to the published AMD tag
"quay.io/gkm/mcv:amd" (replace "quay.io/gkm/mcv:full" with
"quay.io/gkm/mcv:amd") in the example commands so the extract/image flags (e.g.,
the lines showing "--extract --image quay.io/myorg/vllm-cache:v1 --dir /cache")
refer to the correct published AMD image; ensure all instances (including the
ones around the shown diff and the occurrences noted at lines ~117 and ~130) are
changed.

In `@mcv/images/amd64.dockerfile`:
- Around line 98-101: The Dockerfile is downloading the Ubuntu Jammy ROCm .deb
into the Debian Bookworm-based mcv-full stage which is brittle; replace the
Jammy-specific install with the Debian/Bookworm-compatible ROCm install flow:
remove the wget of the jammy URL and the local .deb install steps and instead
configure the official ROCm APT repository for Debian Bookworm (using
ROCM_VERSION/AMDGPU_VERSION variables as needed), apt-get update, and install
the ROCm packages (e.g., amd-smi-lib/rocm-smi-lib) from that repo; update the
RUN commands in mcv/images/amd64.dockerfile that reference the wget line and the
apt install ./*.deb sequence so the image uses the Debian repo-based installer
rather than the Ubuntu Jammy package.
- Around line 21-23: The Dockerfile currently downloads and extracts Go using
the RUN block that references GO_VERSION without verifying the tarball; update
the RUN step that uses wget/tar to also download the official SHA256 checksum
for the matching release, compute the SHA256 of /tmp/go.tgz (e.g., via
sha256sum), compare it to the expected checksum string
(957647d3d78995393c200542ab4c23c72b220c3848b6250787a2d48083818314 for go1.24.6),
and fail the build if mismatched before extracting: keep the same GO_VERSION
variable usage and ensure the verification occurs between the download (wget)
and the tar -xzf step in the RUN that currently removes /tmp/go.tgz.

In `@mcv/README.md`:
- Around line 533-536: The README has trailing whitespace in the updated
paragraph (around the line containing "quay.io/gkm/mcv:latest" /
"quay.io/gkm/mcv:amd" / "quay.io/gkm/mcv:nvidia"); run your project's pre-commit
hooks (or manually remove trailing spaces) to clean up trailing whitespace in
mcv/README.md, then commit the whitespace-only fix so the `trailing-whitespace`
pre-commit check passes.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mcv/docs/unified-mcv-container.md`:
- Around line 44-48: The fenced code block showing runtime detection logic lacks
a language specifier which prevents proper rendering; update the block around
the three lines referencing nvmlCheck(), rocmCheck(), and --no-gpu to include a
language identifier (for example use ```text) immediately after the opening
backticks so the block becomes ```text ... ``` to enable correct formatting and
highlighting.
- Around line 389-399: Update the documented snippet to reflect actual
registration order and logic: mention that registerDevices calls staticCheck(r)
first when config.IsStubEnabled() is true, then calls amdCheck(r), rocmCheck(r),
nvmlCheck(r) sequentially (no short-circuit “first wins”), and note AMD
detection relies on amd-smi via initAMDLib which uses utils.HasApp("amd-smi");
also state that AMD/ROCm exclusivity is handled in addDeviceInterface (AMD
unregisters ROCm and ROCm is skipped if AMD already registered).

---

Outside diff comments:
In `@mcv/images/amd64.dockerfile`:
- Around line 118-128: Replace the obsolete package name libgpgme11 with
libgpgme11t64 in the apt-get install command in the Dockerfile RUN block (the
line that currently installs libgpgme11 along with libbtrfs0, libffi8, libc6,
etc.) so the image matches the Ubuntu 24.04 (Noble) package names used by
mcv-unified and avoids apt install failures on the
nvcr.io/nvidia/cuda:12.6.3-base-ubuntu24.04 base.

In `@mcv/Makefile`:
- Around line 197-207: The image-push target currently pushes minimal, amd,
nvidia and latest tags but omits the unified tag; update the image-push recipe
(target: image-push) to also push
$(IMAGE_REGISTRY)/$(IMAGE_REPOSITORY)/$(IMAGE_NAME):unified (e.g., add a
$(CONTAINER_RUNTIME) push ...:unified line alongside the other tag pushes) so
the :unified artifact produced by image-unified is published.

---

Duplicate comments:
In `@mcv/images/amd64.dockerfile`:
- Around line 98-104: The Dockerfile currently adds Jammy ROCm packages onto
Debian Bookworm/Ubuntu 24.04 (see RUN wget ... amdgpu-install_${AMDGPU_VERSION}
and subsequent apt installs), which is unsupported and brittle; update the
mcv-amd and mcv-unified build stages to use an ROCm-supported base image (e.g.,
an Ubuntu Jammy or official ROCm base image matching
ROCM_VERSION/OPT_ROCM_VERSION) instead of layering Jammy repos onto
Bookworm/Noble, remove the cross-distro apt repo additions and foreign-package
installs (the RUN wget/apt install ./*.deb and ln -s steps), and ensure
ROCM_VERSION/AMDGPU_VERSION/OPT_ROCM_VERSION are aligned with the chosen base so
apt update/apt install resolve natively; rebuild and verify amd-smi/rocm-smi
binaries exist in /opt/rocm-${OPT_ROCM_VERSION}/bin in those stages.

---

Nitpick comments:
In `@mcv/docs/unified-mcv-container.md`:
- Around line 20-28: The document repeatedly recommends using --privileged and
privileged: true without warnings; add a short security notice near the top of
unified-mcv-container.md (after the Basic Usage block) that explains the
security implications of --privileged, calls out that it bypasses
AppArmor/SELinux/seccomp and grants host device access, and states that
--privileged is only required for certain operations (e.g., buildah-based cache
creation) while read-only or runtime GPU usage may be satisfied with targeted
alternatives like --device flags or reduced capabilities; also annotate the
other occurrences of --privileged / privileged: true in the file (the sections
around lines 59-103 and 115-204) to recommend these alternatives and clarify
when --privileged is necessary versus when device-level access suffices.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mcv/docs/unified-mcv-container.md`:
- Around line 44-48: Trailing spaces in the markdown lines containing
"nvmlCheck()", "rocmCheck()", and "--no-gpu" are causing the pre-commit
trailing-whitespace hook to fail; open the markdown, remove any trailing spaces
at the ends of those lines (and the additional trailing spaces reported around
lines 99–103), save, and recommit so the pre-commit hook and CI will pass.

---

Outside diff comments:
In `@mcv/Makefile`:
- Around line 198-207: The image-push target is missing a push for the unified
tag created by image-unified, so add a push for
$(IMAGE_REGISTRY)/$(IMAGE_REPOSITORY)/$(IMAGE_NAME):unified to image-push;
update the image-push recipe (target image-push) to include a
$(CONTAINER_RUNTIME) push invocation for the :unified tag alongside :minimal,
:amd, :nvidia, and :latest so the :unified image in the registry stays current
with the image-unified build.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Makefile`:
- Around line 354-361: Update the Makefile’s deploy image selection so NO_GPU
takes precedence when both NO_GPU and KIND_CLUSTER are true, including the flow
around prepare-deploy. Update kind-load-images to load the same no-GPU image
variables selected for deployment, ensuring KIND uses the newly built no-GPU
images consistently.

In `@mcv/docs/unified-mcv-container.md`:
- Around line 119-140: Update mcv/docs/unified-mcv-container.md at lines 119-140
to add --no-gpu for CPU-node workflows, or explicitly restrict the Job to GPU
nodes. At lines 343-355, remove the automatic no-GPU fallback claim and state
that users must provide --no-gpu. At lines 419-425, qualify CPU-only support by
requiring explicit --no-gpu.

In `@mcv/images/entrypoint.sh`:
- Line 33: Update the entrypoint command around exec so non-MCV commands such as
bash -lc are passed through to their intended executable instead of being
forwarded to /mcv as arguments. Preserve the prior executable passthrough
behavior and ensure the Docker and Podman invocation examples continue working
without requiring entrypoint overrides.

In `@mcv/pkg/cache/vllm_test.go`:
- Around line 324-325: Update the test setup around config.SetEnabledGPU(false)
to capture the existing value from config.IsGPUEnabled() before changing it,
then have the t.Cleanup callback restore that captured value instead of
unconditionally enabling GPU.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@Containerfile.gkm-agent.unified`:
- Around line 74-76: Restore non-root execution for the unified GPU images by
updating the user configuration in Containerfile.gkm-agent.unified at lines
74-76 for /agent and Containerfile.gkm-extract.unified at lines 72-74 for
/gkm-extract. Grant GPU device-group access through deployment configuration or
another constrained mechanism so hardware detection continues without running
either application as root.

In `@mcv/docs/unified-mcv-container.md`:
- Around line 99-107: Update the “Compatibility Check” example in
unified-mcv-container.md to use the currently supported compatibility workflow
and remove the unsupported --check-compat flag, unless the CLI is explicitly
extended to implement it. Keep the documented command aligned with the options
exposed by the mcv CLI help, including the existing --create and --extract
commands.
- Around line 190-195: Update the command block to invoke the installed binary
at /mcv instead of relying on mcv being available on PATH, while preserving the
existing --extract arguments and cache directory.
- Around line 55-64: Update mcv/docs/unified-mcv-container.md:55-64 so the
--create example pushes or exports the generated OCI image before the --rm
container exits, or persists the Buildah store. At
mcv/docs/unified-mcv-container.md:18-23, mount a host volume at /cache so
extracted files survive container removal. At
mcv/docs/unified-mcv-container.md:257-264, show pushing from the creating
container or persisting and sharing its image store with the host.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/Architecture.md`:
- Around line 33-35: Update the “Kernel Cache Extraction and CSI Mounting
Behavior” section to match the documented current flow: the GKM Agent should
launch the gkm-extract/gkm-kernel-cache-extract Kubernetes Job rather than
directly downloading and extracting images. If the direct-extraction behavior is
still supported, explicitly label it as a separate legacy path.

In `@mcv/docs/no-gpu-usage.md`:
- Around line 44-46: Update the no-GPU usage examples around the `podman run`
commands, including the referenced command blocks, so the created OCI image is
persisted or pushed before `--rm` removes the container. Use an in-container
push, `--builder docker` with a trusted daemon, or a shared/persisted image
store, and ensure subsequent host-side push steps do not rely on the
container-internal Buildah store.

In `@mcv/docs/unified-mcv-container.md`:
- Around line 150-153: Update the Kubernetes Job manifest around the cache
volume to preserve extracted cache data beyond pod deletion by replacing
emptyDir with an appropriate PVC or hostPath, and document or configure the
consuming workload that reads it. If the cache is intentionally temporary,
explicitly state that lifecycle instead and remove any implication that the
output persists.
- Around line 313-325: Update the direct Docker command in the “Build the No-GPU
Image (arm64/mac)” section to target linux/arm64 instead of linux/amd64, while
preserving the existing make command and image target details.

---

Outside diff comments:
In @.github/workflows/image-build.yml:
- Line 164: Update the no-GPU agent and extraction build platform configuration
in .github/workflows/image-build.yml at lines 164-164 to publish both
linux/amd64 and linux/arm64, while keeping unified variants amd64-only. Update
the no-GPU MCV build platform configuration in
.github/workflows/mcv-build-image.yml at lines 113-113 similarly; no other
variants require changes.

---

Nitpick comments:
In `@mcv/cmd/main_test.go`:
- Around line 76-97: The regression test TestConfigureBoolFlagsNoGPU only covers
configureBoolFlags directly and does not verify --create dispatch ordering. Add
a dispatch-level test exercising --create with --no-gpu, stubbing or injecting
the create operation and asserting GPU detection is disabled; alternatively,
narrow the test comment to describe only configureBoolFlags unit coverage.

In `@mcv/docs/unified-mcv-container.md`:
- Around line 18-24: Update the extraction-only Podman examples to avoid
--privileged unless MCV extraction demonstrably requires it; use only the
necessary vendor-specific GPU device flags, matching the no-gpu-usage guidance.
If full privilege is required, document the exact capability or device
requirement and retain it only where necessary across all affected examples.

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@gkm-extract/main.go`:
- Around line 82-87: Update the extraction flow in main around clearDirectory
and os.WriteFile: return immediately when clearDirectory(cacheDir) fails instead
of logging and continuing, and likewise return the os.WriteFile error when
creating initFileTmp fails. Ensure extraction does not proceed or mark the cache
as initialized after either failure.
- Around line 73-74: Add a per-cache-directory lock around the complete
extraction transaction in LaunchJob, using a lock path derived from cacheDir and
acquiring it before the existing initialization check and file writes. Hold the
lock through extraction and initialization completion, then release it on every
exit path, ensuring concurrent controller instances cannot write the same
cacheDir.

---

Outside diff comments:
In `@mcv/images/Containerfile`:
- Line 80: Update the image entrypoint configuration around ENTRYPOINT and
entrypoint.sh so invocations whose arguments begin with flags prepend /mcv
before execution, while preserving explicit command execution. Add CMD ["/mcv"]
as the default runtime target, ensuring supplied flag arguments are handled by
the wrapper rather than replacing the command.
- Around line 114-125: Update the AMDGPU installation RUN step to verify the
downloaded installer with a project-controlled SHA-256 pin or equivalent
authenticity check before any installation occurs. Store it at a fixed package
path and install that explicit path rather than using the broad ./*.deb glob,
while preserving the existing cleanup behavior.